-
Notifications
You must be signed in to change notification settings - Fork 521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix part of #4391: Fix failing build on m1 macs #5111
Fix part of #4391: Fix failing build on m1 macs #5111
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kkmurerwa! This PR LGTM. Just one thing: could you change both the title and description to "Fix Part of"? Otherwise the issue will be closed, yet it is not fully resolved.
Unassigning @adhiamboperes since they have already approved the PR. |
One more thing, please refer to #5108 (comment) on how to fix your failing TODO check. |
Hi @seanlip. This is PR ready for another pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Thanks @kkmurerwa!
Hi @kkmurerwa, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kkmurerwa, this LGTM
Unassigning @adhiamboperes since they have already approved the PR. |
Hi @kkmurerwa, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks! |
Fix part of #4391 : Fix failing build on M1 Macs
Explanation
Some M1 Macs fail to build the Oppia app because of unused imports in some of the proto files in the project. This PR attempts to fix this issue by removing unused imports in proto files. Files affected by this fix are;
This is a temporary fix and more investigation will need to be done to properly understand why this behavior is erratic and why it only happens in some instances and not in others. More checks are also needed to ensure that proto files only have imports that are used in the proto file before they are merged.
Besides build fixes, this PR also;
buildifier_download.sh
which previously did not work for MacOS users.static_checks.sh
file to allow contributors to run all static checks locally before pushing the code to Git.Essential Checklist